Skip to content

feat: Add topic SqlFilterCount and CorrelationFilterCount runtime properties - #50022

Open
Eldert Grootenboer (EldertGrootenboer) wants to merge 5 commits into
mainfrom
feature/servicebus-topic-filter-counts-37720586
Open

feat: Add topic SqlFilterCount and CorrelationFilterCount runtime properties#50022
Eldert Grootenboer (EldertGrootenboer) wants to merge 5 commits into
mainfrom
feature/servicebus-topic-filter-counts-37720586

Conversation

@EldertGrootenboer

Copy link
Copy Markdown
Member

Description

Adds the topic-level SqlFilterCount and CorrelationFilterCount runtime properties to the Service Bus administration client, porting the merged .NET implementation (Azure/azure-sdk-for-net#61559) to Java.

API additions

  • TopicRuntimeProperties.getSqlFilterCount() and getCorrelationFilterCount() — the total number of SQL / correlation filters across all of a topic's subscriptions, populated by getTopicRuntimeProperties / getTopicsRuntimeProperties.
  • ServiceBusServiceVersion.V2024_05, now the latest. The administration client sends api-version=2024-05 by default, which the service requires to serve the filter counts. Existing operations are unaffected, and callers can still pin an earlier version.

Behavior

  • The counts are served by the 2024-05 service API version and by regions that have deployed the feature. When absent (older api-version or region), they default to 0.

Code generation

  • The two properties are injected into the generated TopicDescription via a swagger directive, because the pinned 2021-05 input swagger does not yet define them. A regeneration therefore reproduces the fields (verified by running autorest locally — the regenerated TopicDescription matches the committed file). The directive is to be removed once the input-file is bumped to a spec revision that defines the properties.

Tests

  • Unit: service-version default + api-version string mapping; runtime-property propagation + default-to-zero-when-absent; XML serialize/deserialize round-trip.
  • Live: creates a topic with a SQL rule and a correlation rule and asserts SqlFilterCount == 2 (the $Default TrueFilter + the explicit SQL rule) and CorrelationFilterCount == 1. Validated against a live namespace.

Checklist

  • CHANGELOG entry added
  • Unit tests pass
  • Live-validated

…perties

- Add getSqlFilterCount() and getCorrelationFilterCount() to TopicRuntimeProperties,
  exposing the total number of SQL and correlation filters across all of a topic's
  subscriptions
- Add ServiceBusServiceVersion.V2024_05 and make it the latest; the administration
  client now sends api-version=2024-05 by default, which the topic filter counts require
- Parse SqlFilterCount/CorrelationFilterCount ATOM elements onto TopicDescription and
  default to 0 when absent (older region or api-version)
- Add a swagger codegen directive so a regeneration reproduces the two fields instead of
  dropping them
- Add unit tests (service-version default and mapping, propagate and default-to-zero, XML
  round-trip) and a live integration test asserting the counts
…pic-filter-counts-37720586

# Conflicts:
#	sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the Azure Service Bus administration client to surface topic-level runtime counts for SQL and correlation filters, and updates the default administration api-version to 2024-05 to enable the service to return these new fields.

Changes:

  • Added sqlFilterCount and correlationFilterCount plumbing from generated TopicDescriptionTopicProperties → public TopicRuntimeProperties.
  • Introduced ServiceBusServiceVersion.V2024_05 and made it the default/latest service version.
  • Added unit and live integration coverage for version mapping, runtime-property propagation/defaulting, and XML serialization/deserialization.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
sdk/servicebus/azure-messaging-servicebus/swagger/README.md Injects SqlFilterCount / CorrelationFilterCount into TopicDescription via an AutoRest directive for the pinned swagger.
sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusServiceVersionTest.java Verifies latest version selection and enum → api-version string mapping.
sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/implementation/ServiceBusManagementSerializerTest.java Adds XML round-trip test coverage for the new topic filter-count elements.
sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/administration/TopicRuntimePropertiesTest.java Validates propagation to public TopicRuntimeProperties and default-to-zero behavior when absent.
sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/administration/ServiceBusAdministrationClientIntegrationTest.java Adds a live-only test that creates rules and validates the returned topic-level filter counts.
sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusServiceVersion.java Adds V2024_05 and updates getLatest() to return it.
sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/administration/models/TopicRuntimeProperties.java Adds public getters for SQL/correlation filter counts.
sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/administration/models/TopicProperties.java Captures filter counts from TopicDescription and exposes internal getters for runtime model construction.
sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/administration/implementation/models/TopicDescription.java Adds generated fields, XML (de)serialization, and accessors for the new counts.
sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md Documents the new runtime properties and service-version default change.

Comment thread sdk/servicebus/azure-messaging-servicebus/swagger/README.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/administration/ServiceBusAdministrationClientIntegrationTest.java:552

  • This integration test is currently gated to TestMode.LIVE only, so it will be skipped in both RECORD (when recordings are created) and PLAYBACK (the default CI mode for proxy-based tests). That makes the test effectively non-executing in normal runs and increases the risk that it silently breaks over time.

Consider allowing RECORD as well (recordings can be captured from a feature-enabled namespace), and optionally later enabling PLAYBACK once a stable recording exists.

        assumeTrue(super.getTestMode() == TestMode.LIVE, "Filter counts require a live feature-enabled namespace.");

The default administration api-version is now 2024-05 (getLatest()), which the
recorded playback tests would send, but their session recordings were captured
at 2021-05, so the test proxy could not match the requests.

- Pin the recorded (playback/record) modes to ServiceBusServiceVersion.V2021_05
  in the shared configure() and the impl-client and unauthorized-client builders;
  live mode keeps the latest default.
- Mark getTopicFilterCounts with @liveonly so it skips cleanly in playback (it
  needs the 2024-05 default and a feature-enabled live namespace, and has no
  recording), instead of failing setup on a missing recording.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
34 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I checked out this branch locally, built the module, and ran the affected tests to verify correctness, and compared against the merged .NET counterpart (Azure/azure-sdk-for-net#61559) for parity.

Verified locally:

  • mvn compile / test-compile clean, no errors
  • checkstyle:check clean
  • New unit tests (ServiceBusServiceVersionTest, TopicRuntimePropertiesTest, XML round-trip in ServiceBusManagementSerializerTest) all pass
  • Full playback-mode integration suites (ServiceBusAdministrationClientIntegrationTest 45/45, ServiceBusAdministrationAsyncClientIntegrationTest 47/47, ServiceBusAdministrationClientImplIntegrationTests 5/5) all pass, confirming the V2021_05 pin for recorded modes doesn't break existing cassettes
  • XML element names (SqlFilterCount/CorrelationFilterCount) match the merged .NET implementation exactly, low risk of a wire-format mismatch
  • ServiceBusServiceVersion is only referenced by the admin client builder, so bumping the default doesn't touch the AMQP data-plane
  • Author confirmed getTopicFilterCounts and the 2024-05 default were also validated against a live Service Bus namespace, addressing correctness of the new behavior end-to-end

Strengths: clean generated-code diff mirroring existing subscriptionCount patterns, good default-to-zero test coverage, correct null-handling via toPrimitive, swagger directive well-documented and consistent with sibling directives in the same file.

Suggestions (non-blocking):

  1. getTopicFilterCounts is @LiveOnly, so it won't automatically re-run in standard CI (PLAYBACK) or even RECORD going forward. Correctness was manually validated live for this PR, but there's currently no automated regression safety net for this path afterward. Consider allowing RECORD mode once a feature-enabled namespace is consistently available, so a cassette can eventually back a PLAYBACK run.
  2. The .NET counterpart PR added a second, explicit CHANGELOG line under "Other Changes" calling out the default administration service-version bump (2021-05 -> 2024-05) as a behavior change, separate from the "Features Added" bullet. Consider mirroring that here for visibility, since this changes the default wire behavior for all callers who don't pin a version.
  3. Minor nit: the new CHANGELOG bullets don't include a PR reference link (([#NNNNN](...))), unlike the sibling entry above them in the same section.

Nothing here blocks merge - solid, well-tested, low-blast-radius port of the .NET feature.

- Build the client for getTopicFilterCounts with an explicit
  ServiceBusServiceVersion.V2024_05 instead of relying on the builder
  default, matching the merged .NET counterpart.
- Add a getClient(ServiceBusServiceVersion) overload that applies the
  version after configure(), and have the no-arg getClient() delegate to
  it so the setup is not duplicated.
- Correct the comment on why the test is live only: the recorded modes
  are pinned to 2021-05 to match the existing cassettes, and no cassette
  covers this path yet.
- Add an Other Changes entry recording that the default service version
  used by ServiceBusAdministrationClientBuilder moved from 2021-05 to
  2024-05, and how callers pin the previous behavior.
- Mirrors the merged .NET counterpart, which records the same bump
  separately from the Features Added bullet.
@EldertGrootenboer

Copy link
Copy Markdown
Member Author

Your RECORD question turned up that the test was taking the builder default, so a recording would have captured zeros.
It now builds an explicit V2024_05 client, and live the counts come back 2 and 1 at 2024-05 against 0 at 2021-05.
Re-recording also needs configure() moved off its 2021-05 pin plus an assets tag push, so it stays live-only here.

Added the Other Changes entry for the default admin version going to 2024-05, including how callers pin the previous behavior.

On the reference links, most bullets in this package go without one, 4 of the last 18 content bullets carry one and the .NET bullets carry none, so I have left them off.
The sibling above is one of the four.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants